Improved asset bed relations for camera preset #2387
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2387 +/- ##
===========================================
+ Coverage 65.62% 65.98% +0.36%
===========================================
Files 223 227 +4
Lines 13398 13482 +84
Branches 1856 1856
===========================================
+ Hits 8792 8896 +104
+ Misses 4222 4195 -27
- Partials 384 391 +7 ☔ View full report in Codecov by Sentry. |
|
@rithviknishad Is this good to be tested/reviewed? Can you update the issue? |
--------- Co-authored-by: Aakash Singh <[email protected]>
|
LGTM |
|
This is a very very specific implementation, we should try to avoid creating features like this as much as possible. Care needs more abstract thinking. |
| AssetBed.objects.filter(id__in=assetbeds_to_delete).update( | ||
| deleted=True, meta={} | ||
| ) | ||
| AssetBed.objects.filter(deleted=False, asset__asset_class="ONVIF").update( |
There was a problem hiding this comment.
Migrations usually delete data later on, deleting data right away is usually not the best approach.
There was a problem hiding this comment.
Will remove it from this PR and create separate PR to remove it.
There was a problem hiding this comment.
The duplicate asset-bed records would have to be marked as deleted for the unique constraint to be applied, and for the front-end to know which asset-bed record to link to when the preset is being created.
We can however skip deleting the meta data from these old records so that data is still preserved, just that duplicate asset-bed link records would no longer be accessible by the viewset.
| is_migrated=True, | ||
| ) | ||
| ) | ||
| if asset_bed.row_number != 1: |
There was a problem hiding this comment.
We need to preserve the first asset-bed link since the camera preset would be associated to the asset-bed link. So far each old preset had it's own asset-bed record. However, asset-bed records would be now unique together for asset and bed and the newly created preset record would be linked to this asset bed, hence we need to preserve the first asset-bed.
| CameraPreset = apps.get_model("facility", "CameraPreset") | ||
|
|
||
| paginator = Paginator( | ||
| AssetBed.objects.annotate( |
There was a problem hiding this comment.
Do we have that many assetbed realtions in prod right now ?
|
Also, i don't see any tests for this feature. |
Proposed Changes
Associated Issue
Architecture changes
Merge Checklist
/docsOnly PR's with test cases included and passing lint and test pipelines will be reviewed
@coronasafe/care-backend-maintainers @coronasafe/care-backend-admins